Skip to content

Create a prototype test plan - #948

Draft
dbutenhof wants to merge 1 commit into
vllm-project:mainfrom
dbutenhof:doc/testplan
Draft

Create a prototype test plan#948
dbutenhof wants to merge 1 commit into
vllm-project:mainfrom
dbutenhof:doc/testplan

Conversation

@dbutenhof

@dbutenhof dbutenhof commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

0.7.0 broke several things, and we were sloppy about testing. 0.7.1 fixed JSON output with an api_key, but not CSV output. We need to be more disciplined about pre-release testing.

Details

I've considered trying to build a formal manual test plan for some time; a few recent issues brought that back to mind, and I thought I'd make a try to start something.

I began with a few bullet items and thought I'd send that out for review, when the idea struck me to feed my bullet list and some general instructions into Cursor and see what it came up with.

This is the result. I haven't even reviewed it in great detail, but at a glance most of this seems plausible, if not complete. At least, it seems like a place to start; and I like the idea of "enshrining" this in our documentation. Although, ideally, we'll migrate this into our CI, extending or replacing our e2e tests.

Test Plan

  • yes, we should test the test plan!

Related Issues

Related to all issues, closes none!


  • "I certify that all code in this PR is my own, except as noted below."

Use of AI

  • Includes code generated or substantially modified by an AI agent
  • Includes tests generated or substantially modified by an AI agent

NOTE: the Generated-by or Assisted-by trailers should be used in git commit messages when code or tests were generated or substantially modified by an AI agent, as described in the project's DEVELOPING.md file.


git log

commit 9ff2878
Author: David Butenhof dbutenho@redhat.com
Date: Wed Jul 22 16:37:02 2026 -0400

Create a prototype test plan

Signed-off-by: David Butenhof <dbutenho@redhat.com>

Signed-off-by: David Butenhof dbutenho@redhat.com

Signed-off-by: David Butenhof <dbutenho@redhat.com>
@dbutenhof dbutenhof self-assigned this Jul 22, 2026
@dbutenhof dbutenhof added documentation Improvements or additions to documentation internal filed by core contributor or associate labels Jul 22, 2026
@dbutenhof dbutenhof added this to the v0.8.0 milestone Jul 25, 2026

@sjmonson sjmonson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I think this is a good idea, though maybe I would label it as "release smoke test plan" since it does not go into performance testing. I didn't verify any of the examples but they look fine. Some minor nits below:

--output kind=plot,path="${OUT}/run.png",dpi=100
```

**B. `guidellm benchmark from-file`**

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this guidellm export now?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah -- this went up before Jared made that change.

Comment on lines +474 to +480
## Known gaps (context for testers)

- `kind=plot` is implemented but not yet documented in the outputs guide; always pass an explicit `path=` for release runs.
- Websocket realtime ASR has little user documentation; rely on this checklist and code/tests.
- WEKA / OTEL trace replay is intentionally excluded until implementation lands.
- `vllm_offline` may be absent from the candidate—record N/A rather than Fail.
- Hugging Face multimodal paths are thinly covered by automated e2e; this manual matrix is the primary confidence gate for Hub datasets.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These don't seem to helpful:

  • kind=plot is implemented but not yet documented in the outputs guide; always pass an explicit path= for release runs.
  • Websocket realtime ASR has little user documentation; rely on this checklist and code/tests.

Shouldn't we just fix the missing documentation?

  • WEKA / OTEL trace replay is intentionally excluded until implementation lands.
  • vllm_offline may be absent from the candidate—record N/A rather than Fail.

Is it really a gap that we don't test things that don't exist yet?

  • Hugging Face multimodal paths are thinly covered by automated e2e; this manual matrix is the primary confidence gate for Hub datasets.

This is not really saying anything.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah -- Cursor, interestingly, analyzed outstanding PRs rather than just the current branch, which surprised me, and anticipated with a lot of additional test cases we can't yet perform. I corrected it in a few cases, or just edited the output; but in retrospect I should probably have just deleted this whole section.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that as well. This might also lead to having to regularly read through and update this doc to keep it up to date.

| 14 | Embeddings API | | | |
| 15 | `vllm_python` (normal) | | | |
| 16 | `vllm_offline` (batch) | | | Skip if not in release |
| 17 | `trace_synthetic` (optional) | | | |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why optional?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question. I infer that Cursor thought of this as a clean subset of Mooncake replay that didn't add much on its own (note that the trace_synthetic test above is "Optional prelude") -- but that seems questionable logic to me since Mooncake relies on the hash-id sequences which are missing here.

Comment on lines +18 to +22
# From PyPI / wheel under test
pip install "guidellm[recommended,vision,audio,plot]"

# Or from a local checkout
uv sync --extra recommended --extra vision --extra audio --extra plot

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just all?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were generated by Cursor, and I only glanced through quickly before posting to give everyone a chance at it. But, yeah; it's clearly made some questionable choices and I probably shouldn't have been so quick to post. 😆

Comment on lines +18 to +22
# From PyPI / wheel under test
pip install "guidellm[recommended,vision,audio,plot]"

# Or from a local checkout
uv sync --extra recommended --extra vision --extra audio --extra plot

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were generated by Cursor, and I only glanced through quickly before posting to give everyone a chance at it. But, yeah; it's clearly made some questionable choices and I probably shouldn't have been so quick to post. 😆

--output kind=plot,path="${OUT}/run.png",dpi=100
```

**B. `guidellm benchmark from-file`**

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah -- this went up before Jared made that change.

| 14 | Embeddings API | | | |
| 15 | `vllm_python` (normal) | | | |
| 16 | `vllm_offline` (batch) | | | Skip if not in release |
| 17 | `trace_synthetic` (optional) | | | |

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question. I infer that Cursor thought of this as a clean subset of Mooncake replay that didn't add much on its own (note that the trace_synthetic test above is "Optional prelude") -- but that seems questionable logic to me since Mooncake relies on the hash-id sequences which are missing here.

Comment on lines +474 to +480
## Known gaps (context for testers)

- `kind=plot` is implemented but not yet documented in the outputs guide; always pass an explicit `path=` for release runs.
- Websocket realtime ASR has little user documentation; rely on this checklist and code/tests.
- WEKA / OTEL trace replay is intentionally excluded until implementation lands.
- `vllm_offline` may be absent from the candidate—record N/A rather than Fail.
- Hugging Face multimodal paths are thinly covered by automated e2e; this manual matrix is the primary confidence gate for Hub datasets.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah -- Cursor, interestingly, analyzed outstanding PRs rather than just the current branch, which surprised me, and anticipated with a lot of additional test cases we can't yet perform. I corrected it in a few cases, or just edited the output; but in retrospect I should probably have just deleted this whole section.

Comment thread docs/developer/release-test-plan.md

@SkiHatDuckie SkiHatDuckie left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some things I found. Main gripe is with the "xyz is missing/in active development" notes, as this doc will have to be regularly rechecked to see if notes need to be added or deleted.

| `console` | Metadata / info / stats tables print without traceback |
| `json` | Valid JSON; contains benchmarks and metrics |
| `csv` | Non-empty summary rows |
| `html` | Opens in a browser; tables/charts render |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The HTML output for me has consistently returned an empty file (I'm assuming this is due to the output type not being maintained). I also don't seem to be the only one who has experienced this: #941 . Unless the state of the HTML option changes, this is a bit misleading.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I've always seen it generate a file with data; but until Sam's recent fix it just didn't render. The fix does require a new HTML file generated with the GuideLLM fix since it's really just locking to a specific github.io URL. But I just generated a new html from main, and it renders just fine.

On the other hand, the description wording here loosely suggests that something will magically open the HTML in a browser, while obviously you need to do that manually.

Comment on lines +424 to +426
{"timestamp": 0.0, "input_length": 10, "output_length": 5, "hash_ids": [0]}
{"timestamp": 0.2, "input_length": 20, "output_length": 8, "hash_ids": [0, 1]}
{"timestamp": 0.4, "input_length": 15, "output_length": 6, "hash_ids": [2]}

@SkiHatDuckie SkiHatDuckie Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hash IDs here are malformed. With the Mooncake default block size, you'd need at least 513 input tokens to get 2 hash IDs. You could either set the hash_id_block_size to something like 15, or remove either the 0 or 1 in the second row. Both should work.

Edit: Or increase the input_length to something between 512 and 1024.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks ... I posted this without trying to proof-read in depth, although it's possible I might not have noticed that detail anyway. 😆

--output kind=json,path=results/mooncake.json
```

Optional column overrides: `hash_ids_column`, `hash_id_block_size` (default `512`), plus the shared `timestamp_column` / `prompt_tokens_column` / `output_tokens_column`.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--pedantic, but hash_id_block_size isn't a column override.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I like Cursor's term "override" anyway here -- thanks for pointing that out.

Comment on lines +474 to +480
## Known gaps (context for testers)

- `kind=plot` is implemented but not yet documented in the outputs guide; always pass an explicit `path=` for release runs.
- Websocket realtime ASR has little user documentation; rely on this checklist and code/tests.
- WEKA / OTEL trace replay is intentionally excluded until implementation lands.
- `vllm_offline` may be absent from the candidate—record N/A rather than Fail.
- Hugging Face multimodal paths are thinly covered by automated e2e; this manual matrix is the primary confidence gate for Hub datasets.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that as well. This might also lead to having to regularly read through and update this doc to keep it up to date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation internal filed by core contributor or associate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants